chore(native): declare PyContextVar_* locally for limited-API builds (PROF-15850) - #19903
chore(native): declare PyContextVar_* locally for limited-API builds (PROF-15850)#19903vlad-scherbich wants to merge 1 commit into
Conversation
|
BenchmarksBenchmark execution time: 2026-08-30 18:56:06 Comparing candidate commit 72d7f7a in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 575 metrics, 10 unstable metrics, 2 known flaky benchmarks, 16 flaky benchmarks without significant changes.
|
|
Part of #19908 |
There was a problem hiding this comment.
Pull request overview
This PR fixes Rust native extension builds under Py_LIMITED_API (e.g., CPython 3.15 builds using PYO3_USE_ABI3_FORWARD_COMPATIBILITY) by locally declaring the PyContextVar_New, PyContextVar_Get, and PyContextVar_Set C-API entry points that are not exposed by pyo3-ffi in that configuration.
Changes:
- Add local
extern "C"declarations forPyContextVar_*functions insrc/native/contextvar.rs. - Switch call sites from
ffi::PyContextVar_*to the locally declared symbols so the module compiles with and withoutPy_LIMITED_API.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
5e2e905 to
9c1783d
Compare
Codeowners resolved asResolved from the full PR diff against |
Circular import analysis
|
Dependency direction analysis
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e2e90539b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f668305 to
def36ad
Compare
f9ee924 to
12acbff
Compare
12acbff to
59047aa
Compare
59047aa to
bb9aaf9
Compare
4362679 to
c493409
Compare
bb9aaf9 to
c2e9f4f
Compare
cac61ca to
284bca6
Compare
…imited-API PyContextVar_New/_Get/_Set are absent from pyo3-ffi limited-API bindings. PyGILState_Check is not limited-API; on those builds skip the probe rather than acquire or guess from a crash signal handler.
72d7f7a to
792f843
Compare
There was a problem hiding this comment.
@gyuheon0h can you please take a look if this change makes sense here as part of the PR stack?
There was a problem hiding this comment.
@vlad-scherbich update summary, add an example crash this is fixing.
emmettbutler
left a comment
There was a problem hiding this comment.
Seeing an example of the failure that occurs without this change would be helpful. Do we build with multiple values of the FORWARD_COMPATIBILITY flag? Why is it necessary to support multiple values of that flag?
There was a problem hiding this comment.
@vlad-scherbich update summary, add an example crash this is fixing.
Description
Delta vs #19861.
src/native/contextvar.rsdeclaresPyContextVar_New/_Get/_Setlocally and calls those instead ofpyo3::ffi. pyo3-ffi omits them from limited-API bindings.If this merges and nothing else new lands: limited-API contextvar builds compile. No publish, testrunner, riot, or
requires-pythonchange.Testing
Native module compiles with the local declarations.
Risks
None. Declarations do not change runtime behavior on 3.9–3.14.
Additional Notes
No release note: internal compile fix,
changelog/no-changelog.Base: #19861. Next: #19907.